fix(clerk-react): Add versionSelector helper - #1780
Conversation
🦋 Changeset detectedLatest commit: ac078a6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
!snapshot |
|
Hey @LekoArts - the snapshot version command generated the following package versions:
Tip: use the snippet copy button below to quickly install the required packages. # @clerk/backend
npm i @clerk/backend@0.29.3-snapshot.08fefec# @clerk/chrome-extension
npm i @clerk/chrome-extension@0.4.2-snapshot.08fefec# @clerk/clerk-js
npm i @clerk/clerk-js@4.58.2-snapshot.08fefec# @clerk/clerk-expo
npm i @clerk/clerk-expo@0.19.4-snapshot.08fefec# @clerk/fastify
npm i @clerk/fastify@0.6.9-snapshot.08fefec# gatsby-plugin-clerk
npm i gatsby-plugin-clerk@4.4.10-snapshot.08fefec# @clerk/localizations
npm i @clerk/localizations@1.26.1-snapshot.08fefec# @clerk/nextjs
npm i @clerk/nextjs@4.24.2-snapshot.08fefec# @clerk/clerk-react
npm i @clerk/clerk-react@4.25.2-snapshot.08fefec# @clerk/remix
npm i @clerk/remix@3.0.1-snapshot.08fefec# @clerk/clerk-sdk-node
npm i @clerk/clerk-sdk-node@4.12.8-snapshot.08fefec# @clerk/shared
npm i @clerk/shared@0.23.1-snapshot.08fefec# @clerk/types
npm i @clerk/types@3.52.1-snapshot.08fefec |
|
Looking at https://unpkg.com/browse/@clerk/clerk-react@4.25.2-snapshot.08fefec/dist/esm/utils/versionSelector.js, the replacement works. It sets the correct |
…uld-load-their-specific-clerkjs
|
|
||
| const prereleaseTag = getPrereleaseTag(PACKAGE_VERSION); | ||
| if (prereleaseTag) { | ||
| if (prereleaseTag === 'staging' || prereleaseTag === 'snapshot') { |
There was a problem hiding this comment.
I definitely agree with the proposed change here for the snapshot environment, but can you please expand a little on why we need the same for the staging environment as well?
There was a problem hiding this comment.
staging (which gets released after PRs are merged into main branch) is also a tagged release. If you want to test the main branch in a certain state, you'll need to make sure that also the clerk-js version is the correct one.
Basically staging has the same problem as snapshot
There was a problem hiding this comment.
I am still slightly confused :) Why do we consider staging to be a tagged release? What would be the use case of testing a staging release using a specific staging version instead of using the most recent @staging package?
Snapshot and staging releases are still different in my mind - of course, I'm very open to discussion. Snapshot releases need to be bound with their corresponding clerk-js version because we can have multiple PRs open, generating snapshot releases simultaneously.
Staging releases behave differently, though. Only a single branch (main) triggers staging releases, so even if multiple staging releases happen each day, this always happens sequentially. Moreover, the main branch should always be deployable, meaning that if you test a staging version that is not the most recent one, you are not testing the code that will actually be deployed.
The main reason to have a staging environment is to catch and address bugs before they reach production. If the staging environment doesn't mirror production, it might not replicate the issues that could arise in the production environment. In our case that'd be:
- hotloading using a tag (eg
@staging) and not a specific version - making sure that the SDKs (eg
clerk-react) works with the most recentclerk-jsrelease
This might sound like a minor detail, however, issues in the clerk-js delivery mechanism might result in downtime especially while hotloading is in place. There are ways to work around this (eg, have automated tests use clerkJSVersion='staging' to emulate loading the most recent staging release) but that'd still not mirror 100% the production behavior.
There was a problem hiding this comment.
I also agree with @nikosdouvlis here. I would keep this change only for the snapshot env.
There was a problem hiding this comment.
@nikosdouvlis @anagstef Made the change :)
…uld-load-their-specific-clerkjs
…uld-load-their-specific-clerkjs
…uld-load-their-specific-clerkjs
…uld-load-their-specific-clerkjs
…uld-load-their-specific-clerkjs
|
This PR has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Description
This PR refactors the
packages/react/src/utils/loadClerkJsScript.tscode a little bit to make theversioneasier to grok. This is achieved by moving the logic to aversionSelectorhelper. While doing that I also added the functionality that forstagingandsnapshotprereleases we use the exactclerk-jsversion, not only the tag.This ensures that when you use a
snapshot/stagingrelease you use the correct/relatedclerk-jsrelease, not the latestsnapshot/stagingresolved version.This is a new feature for us internally so this is only good for a patch version, not a minor.
Checklist
npm testruns as expected.npm run buildruns as expected.Type of change
Packages affected
@clerk/clerk-js@clerk/clerk-react@clerk/nextjs@clerk/remix@clerk/types@clerk/themes@clerk/localizations@clerk/clerk-expo@clerk/backend@clerk/clerk-sdk-node@clerk/shared@clerk/fastify@clerk/chrome-extensiongatsby-plugin-clerkbuild/tooling/chore